User Management Navigator Supervisor Backend Implementation #120

sanjaytkbabu · 2024-07-23T22:58:58Z

Description

PADS-222
User Management - Supervisor backend implementation
features - create new user, revoke user, manage user (will be functional after rbac)
migration added for access_request table

Types of changes

New feature (non-breaking change which adds functionality)

Checklist

I have read the CONTRIBUTING doc
I have checked that unit tests pass locally with my changes
I have added tests that prove my fix is effective or that my feature works
I have added necessary documentation (if appropriate)

Further comments

github-actions · 2024-07-23T22:59:42Z

Coverage Report (Application)

Totals
Statements:	44.58% ( 938 / 2104 )
Methods:	34.02% ( 132 / 388 )
Lines:	55.25% ( 616 / 1115 )
Branches:	31.61% ( 190 / 601 )

github-actions · 2024-07-23T23:00:05Z

Coverage Report (Frontend)

Totals
Statements:	30.91% ( 1454 / 4704 )
Methods:	26.92% ( 249 / 925 )
Lines:	35.42% ( 899 / 2538 )
Branches:	24.66% ( 306 / 1241 )

Subin1Doo · 2024-07-24T16:31:30Z

Things are looking great! Few comments..
For the status can we say "what" it's pending. So it would either say "Pending approval" or "Pending revocation."
And for the Create new user modal, can we make sure everything is left / right aligned. Thanks!

sanjaytkbabu · 2024-07-24T17:01:44Z

Things are looking great! Few comments.. For the status can we say "what" it's pending. So it would either say "Pending approval" or "Pending revocation." And for the Create new user modal, can we make sure everything is left / right aligned. Thanks!

done!

Subin1Doo · 2024-07-24T17:35:33Z

Sorry just double checking, once the pending revocation has been approved by the admin the status should say "revoked" in the Nav supervisor view. "Approved" for new user approval.

sanjaytkbabu · 2024-07-24T17:47:59Z

Sorry just double checking, once the pending revocation has been approved by the admin the status should say "revoked" in the Nav supervisor view. "Approved" for new user approval.

yes, that's correct

kyle1morel · 2024-07-25T19:44:38Z

app/src/db/models/access_request.ts

+
+import type { Stamps } from '../stamps';
+import type { AccessRequest } from '../../types/AccessRequest'; // Import the access_request_status_enum type
+import { UserStatus } from '../../utils/enums/application';


Import ordering

thanks for catching

kyle1morel · 2024-07-25T19:48:34Z

app/src/services/user.ts


 const trxWrapper = (etrx: Prisma.TransactionClient | undefined = undefined) => (etrx ? etrx : prisma);

+// Constants
+const SYSTEM_USER = 'system';


SYSTEM_USER should be checked by it's guid, which is NIL, not name.
const SYSTEM_USER = NIL;

kyle1morel · 2024-07-25T19:52:12Z

app/src/types/User.ts

@@ -10,4 +12,5 @@ export type User = {
  fullName: string | null;
  lastName: string | null;
  active: boolean;
+  accessRequest?: AccessRequest | null;


User and AccessRequest are different concepts. It doesn't feel right that they are combined like this.

kyle1morel · 2024-07-25T19:53:22Z

app/src/services/user.ts

+   * @returns {Promise<object>} The result of running the insert operation
+   * @throws The error encountered upon db transaction failure
+   */
+  createUserAccessRequest: async (data: User) => {


Per lower comment, user and AccessRequest are different concepts. Function should take two different parameters.

kyle1morel · 2024-07-25T19:56:22Z

app/src/services/user.ts

+  /**
+   * @function listNavigators
+   * Lists all the
+   * @param {boolean} [active] Optional boolean on user active status
+   * @returns {Promise<object>} The result of running the find operation
+   */
+  getNavsAndAccessRequests: async () => {


JSDoc needs to be fixed. And function name isn't accurate as it's not limited to just the Navs. getLatestAccessRequests is more appropriate.

kyle1morel · 2024-07-25T19:58:06Z

app/src/services/user.ts

+  /**
+   * @function revokeUserAccessRequest
+   * Updates user access
+   * @returns {Promise<object>} The result of running the put operation
+   */


Incorrect JSDoc

thanks for catching

kyle1morel · 2024-07-25T19:59:22Z

app/src/services/user.ts

+   * @returns {Promise<object>} The result of running the put operation
+   */
+  updateUserRole: async (user: User) => {
+    console.log(user);


Remove console log

I just put it there to stop unused 'user' variable complaints. I've removed it!

kyle1morel · 2024-07-25T20:00:53Z

app/src/utils/enums/application.ts

@@ -30,6 +30,11 @@ export enum Initiative {
  HOUSING = 'HOUSING'
 }

+export enum UserStatus {


Misnomer. This is the status of the access request, not the user.

makes sense

kyle1morel · 2024-07-25T20:01:33Z

app/src/utils/enums/application.ts

+}
 export enum Regex {


Style: Please ensure new lines are between blocks. I've noticed this a few times within this PR.

wilwong89 · 2024-08-01T22:52:10Z

app/src/services/user.ts

+        response = await service.createUser(data, trx);
+      });
+      return user.fromPrismaModel(response);
+    } else return;


Transaction not required at this level.

Subin1Doo · 2024-08-02T23:36:20Z

Comment on the sorting of the "status" column.
When I sort this column, it doesn't really seem to be sorting. The order seems rather random.
Can we make it so that when they sort by "status" it shows the pending approval ones first, then the pending revocation ones, then all the approved ones? and the other way around. Thank you!

sanjaytkbabu · 2024-08-07T15:51:39Z

Comment on the sorting of the "status" column. When I sort this column, it doesn't really seem to be sorting. The order seems rather random. Can we make it so that when they sort by "status" it shows the pending approval ones first, then the pending revocation ones, then all the approved ones? and the other way around. Thank you!

As discussed went with PR, PA & A.

kyle1morel

Reviewed semantically. Lots of file changes, difficult to follow the full logic. Functionality looks to be there.

sanjaytkbabu force-pushed the feature/nav-supervisor-be branch 5 times, most recently from 4755dc1 to 6021eea Compare July 24, 2024 15:24

sanjaytkbabu force-pushed the feature/nav-supervisor-be branch from 6021eea to 92b460a Compare July 24, 2024 16:54

sanjaytkbabu force-pushed the feature/nav-supervisor-be branch from 92b460a to 1472961 Compare July 24, 2024 17:12

sanjaytkbabu force-pushed the feature/nav-supervisor-be branch from 1472961 to 20989b3 Compare July 24, 2024 17:47

sanjaytkbabu marked this pull request as ready for review July 24, 2024 17:59

sanjaytkbabu requested review from kyle1morel, wilwong89 and qhanson55 as code owners July 24, 2024 17:59

sanjaytkbabu requested a review from jujaga July 24, 2024 18:58

sanjaytkbabu force-pushed the feature/nav-supervisor-be branch 2 times, most recently from 678ac54 to b255a96 Compare July 25, 2024 16:23

kyle1morel reviewed Jul 25, 2024

View reviewed changes

sanjaytkbabu force-pushed the feature/nav-supervisor-be branch 2 times, most recently from 094a6cc to 2c4df1f Compare August 1, 2024 17:30

sanjaytkbabu requested a review from kyle1morel August 1, 2024 17:39

sanjaytkbabu force-pushed the feature/nav-supervisor-be branch from 2c4df1f to 5b617b0 Compare August 1, 2024 17:44

sanjaytkbabu closed this Aug 1, 2024

sanjaytkbabu reopened this Aug 1, 2024

sanjaytkbabu force-pushed the feature/nav-supervisor-be branch 2 times, most recently from 6296b20 to 11b21df Compare August 1, 2024 21:09

wilwong89 reviewed Aug 1, 2024

View reviewed changes

sanjaytkbabu force-pushed the feature/nav-supervisor-be branch 2 times, most recently from bae93d4 to cb78639 Compare August 2, 2024 21:44

sanjaytkbabu force-pushed the feature/nav-supervisor-be branch 2 times, most recently from 0899735 to 771576a Compare August 4, 2024 03:08

user mgmt supervisor backend implementation

1b74a32

sanjaytkbabu force-pushed the feature/nav-supervisor-be branch from 771576a to 1b74a32 Compare August 4, 2024 03:17

kyle1morel approved these changes Aug 8, 2024

View reviewed changes

kyle1morel merged commit 0d30d7d into release/user-management Aug 8, 2024
16 of 17 checks passed

kyle1morel deleted the feature/nav-supervisor-be branch August 8, 2024 00:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

User Management Navigator Supervisor Backend Implementation #120

User Management Navigator Supervisor Backend Implementation #120

sanjaytkbabu commented Jul 23, 2024 •

edited

Loading

github-actions bot commented Jul 23, 2024 •

edited

Loading

github-actions bot commented Jul 23, 2024 •

edited

Loading

Subin1Doo commented Jul 24, 2024

sanjaytkbabu commented Jul 24, 2024

Subin1Doo commented Jul 24, 2024

sanjaytkbabu commented Jul 24, 2024

kyle1morel Jul 25, 2024

sanjaytkbabu Jul 25, 2024

kyle1morel Jul 25, 2024

sanjaytkbabu Jul 25, 2024

kyle1morel Jul 25, 2024

kyle1morel Jul 25, 2024

kyle1morel Jul 25, 2024

kyle1morel Jul 25, 2024

sanjaytkbabu Jul 25, 2024

kyle1morel Jul 25, 2024

sanjaytkbabu Jul 25, 2024

kyle1morel Jul 25, 2024

sanjaytkbabu Jul 25, 2024

kyle1morel Jul 25, 2024

sanjaytkbabu Jul 25, 2024

wilwong89 Aug 1, 2024

sanjaytkbabu Aug 7, 2024

Subin1Doo commented Aug 2, 2024

sanjaytkbabu commented Aug 7, 2024

kyle1morel left a comment

User Management Navigator Supervisor Backend Implementation #120

User Management Navigator Supervisor Backend Implementation #120

Conversation

sanjaytkbabu commented Jul 23, 2024 • edited Loading

Description

Types of changes

Checklist

Further comments

github-actions bot commented Jul 23, 2024 • edited Loading

Coverage Report (Application)

github-actions bot commented Jul 23, 2024 • edited Loading

Coverage Report (Frontend)

Subin1Doo commented Jul 24, 2024

sanjaytkbabu commented Jul 24, 2024

Subin1Doo commented Jul 24, 2024

sanjaytkbabu commented Jul 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Subin1Doo commented Aug 2, 2024

sanjaytkbabu commented Aug 7, 2024

kyle1morel left a comment

Choose a reason for hiding this comment

sanjaytkbabu commented Jul 23, 2024 •

edited

Loading

github-actions bot commented Jul 23, 2024 •

edited

Loading

github-actions bot commented Jul 23, 2024 •

edited

Loading